Skip to content

[Prism] Fix panic in teststream - #36227

Merged
shunping merged 1 commit into
apache:masterfrom
shunping:prism-teststream-panic
Sep 21, 2025
Merged

[Prism] Fix panic in teststream#36227
shunping merged 1 commit into
apache:masterfrom
shunping:prism-teststream-panic

Conversation

@shunping

@shunping shunping commented Sep 21, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

I have seen the following panic in one of the precommit test logs ( https://github.com/apache/beam/actions/runs/17888682703/job/50866217521?pr=36226).

 0: panic in ElementManager.Bundles watermark evaluation goroutine: assignment to entry in nil map
goroutine 5592 [running]:
runtime/debug.Stack()
	/home/runner/sdk/go1.24.4/src/runtime/debug/stack.go:26 +0x5e
github.com/apache/beam/sdks/v2/go/pkg/beam/runners/prism/internal/engine.(*ElementManager).Bundles.func2.1()
	/runner/_work/beam/beam/sdks/go/pkg/beam/runners/prism/internal/engine/elementmanager.go:385 +0x39
panic({0x169b4c0?, 0x2d0a4a0?})
	/home/runner/sdk/go1.24.4/src/runtime/panic.go:792 +0x132
github.com/apache/beam/sdks/v2/go/pkg/beam/runners/prism/internal/engine.set[...].insert(...)
	/runner/_work/beam/beam/sdks/go/pkg/beam/runners/prism/internal/engine/elementmanager.go:1131
github.com/apache/beam/sdks/v2/go/pkg/beam/runners/prism/internal/engine.tsFinalEvent.Execute({{0xc153f7fc50?, 0x15c9f40?}}, 0xc212f6bd40)
	/runner/_work/beam/beam/sdks/go/pkg/beam/runners/prism/internal/engine/teststream.go:256 +0xa5
github.com/apache/beam/sdks/v2/go/pkg/beam/runners/prism/internal/engine.(*ElementManager).checkForQuiescence(0xc212f6bd40, 0xc212f6bd40?)
	/runner/_work/beam/beam/sdks/go/pkg/beam/runners/prism/internal/engine/elementmanager.go:542 +0x17b
github.com/apache/beam/sdks/v2/go/pkg/beam/runners/prism/internal/engine.(*ElementManager).Bundles.func2()
	/runner/_work/beam/beam/sdks/go/pkg/beam/runners/prism/internal/engine/elementmanager.go:497 +0x251
created by github.com/apache/beam/sdks/v2/go/pkg/beam/runners/prism/internal/engine.(*ElementManager).Bundles in goroutine 5582
	/runner/_work/beam/beam/sdks/go/pkg/beam/runners/prism/internal/engine/elementmanager.go:381 +0x156

Basically, there are a few cases where updateWatermarks will return a nil set(map). For example:

beam/sdks/go/pkg/beam/runners/prism/internal/engine/elementmanager.go

Line 2101 in 3659832

return nil

For most references of updateWatermarks, the returned map are read not written. The one in teststream, however, is an exception. Within there, we try to insert an element to this returned set(map), which leads to a panic if the set(map) is nil.

beam/sdks/go/pkg/beam/runners/prism/internal/engine/teststream.go

Lines 255 to 256 in 3659832

kickSet := ss.updateWatermarks(em)
kickSet.insert(ev.stageID)

Here is a note from go blog (https://go.dev/blog/maps#declaration-and-initialization):

A nil map behaves like an empty map when reading, but attempts to write to a nil map will cause a runtime panic; don’t do that.

@gemini-code-assist

gemini-code-assist Bot commented Sep 21, 2025

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @shunping, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical bug that caused a runtime panic in the teststream runner, specifically within the ElementManager's watermark evaluation goroutine. The issue stemmed from an attempt to write to a nil map, which could be returned by the updateWatermarks function. The changes introduce a safeguard to initialize the map if it's nil, ensuring robust behavior and preventing application crashes during test execution.

Highlights

  • Panic Fix: Resolved a runtime panic in the teststream component that occurred when attempting to insert an element into a nil map returned by the updateWatermarks function.
  • Nil Map Handling: Introduced a check to ensure that the kickSet map is explicitly initialized if updateWatermarks returns a nil value, preventing illegal write operations.
  • Documentation Update: Updated the documentation for the updateWatermarks function to clarify that it may return a nil set of stages.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

@shunping shunping changed the title Fix panic in teststream [Prism] Fix panic in teststream Sep 21, 2025
@shunping shunping self-assigned this Sep 21, 2025
@github-actions

github-actions Bot commented Sep 21, 2025

Copy link
Copy Markdown
Contributor

Assigning reviewers:

R: @jrmccluskey for label go.

Note: If you would like to opt out of this review, comment assign to next reviewer.

Available commands:

  • stop reviewer notifications - opt out of the automated review tooling
  • remind me after tests pass - tag the comment author after tests pass
  • waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

@shunping
shunping merged commit 354a387 into apache:master Sep 21, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@liferoad liferoad liferoad approved these changes

Assignees

@shunping shunping

Labels

go Next Action: Reviewers prism runners

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@shunping @liferoad